Skip to content

✨: create HasArrayNamepace #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 9, 2025
Merged

Conversation

nstarman
Copy link
Collaborator

@nstarman nstarman commented Jun 7, 2025

Requires #12

@nstarman nstarman changed the title 📝 docs: add and test readm 📝 docs: add and test readme Jun 7, 2025
@nstarman nstarman force-pushed the code/has_namespace branch 2 times, most recently from 9318276 to 7ec8d29 Compare June 7, 2025 21:33
@nstarman nstarman requested a review from jorenham June 7, 2025 21:36
@nstarman nstarman mentioned this pull request Jun 7, 2025
1 task
@nstarman nstarman changed the title 📝 docs: add and test readme ✨: create HasNameppace Jun 7, 2025
@nstarman nstarman changed the title ✨: create HasNameppace ✨: create HasNamepace Jun 7, 2025
@nstarman nstarman marked this pull request as ready for review June 7, 2025 21:38
@nstarman
Copy link
Collaborator Author

nstarman commented Jun 7, 2025

Also, do we want to call it HasNamespace?

@nstarman nstarman force-pushed the code/has_namespace branch from 7ec8d29 to aaebe45 Compare June 7, 2025 22:06
@jorenham
Copy link
Collaborator

jorenham commented Jun 7, 2025

Also, do we want to call it HasNamespace?

From the typing spec:

A few guidelines for protocol names below. In cases that don’t fall into any of those categories, use your best judgement.

  • Use plain names for protocols that represent a clear concept (e.g. Iterator, Container).
  • Use SupportsX for protocols that provide callable methods (e.g. SupportsInt, SupportsRead, SupportsReadSeek).
  • Use HasX for protocols that have readable and/or writable attributes or getter/setter methods (e.g. HasItems, HasFileno).

So I suppose SupportsNamespace would be more in line with that. But AFAIK HasNamespace is also fine, and more concise too.

@nstarman nstarman force-pushed the code/has_namespace branch from aaebe45 to 151ea4b Compare June 7, 2025 22:23
@nstarman nstarman requested a review from jorenham June 7, 2025 22:24
@nstarman nstarman force-pushed the code/has_namespace branch 3 times, most recently from 83637a4 to b6e28a6 Compare June 7, 2025 23:53
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
@nstarman nstarman force-pushed the code/has_namespace branch from b6e28a6 to 53732c8 Compare June 8, 2025 00:25
@nstarman nstarman requested a review from jorenham June 8, 2025 01:18
@nstarman
Copy link
Collaborator Author

nstarman commented Jun 8, 2025

FYI I'm a fan of squash merges, so totally feel free to do that here.

@lucascolley
Copy link
Member

I think I like the sound of HasArrayNamespace

@nstarman
Copy link
Collaborator Author

nstarman commented Jun 8, 2025

SGTM. Done ✅

@nstarman nstarman force-pushed the code/has_namespace branch from 53732c8 to 7c5958f Compare June 8, 2025 20:30
@nstarman nstarman changed the title ✨: create HasNamepace ✨: create HasArrayNamepace Jun 8, 2025
@nstarman nstarman force-pushed the code/has_namespace branch 3 times, most recently from e594225 to d0a03c5 Compare June 8, 2025 21:22
nstarman added 2 commits June 8, 2025 17:28
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
@nstarman nstarman force-pushed the code/has_namespace branch from d0a03c5 to c92ca8c Compare June 8, 2025 21:28
jorenham
jorenham previously approved these changes Jun 9, 2025
@jorenham jorenham enabled auto-merge (squash) June 9, 2025 01:29
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
@nstarman
Copy link
Collaborator Author

nstarman commented Jun 9, 2025

@jorenham addressing #17 (comment) dismissed your review.

@jorenham jorenham merged commit 1d187c1 into data-apis:main Jun 9, 2025
@jorenham
Copy link
Collaborator

jorenham commented Jun 9, 2025

Thanks @nstarman @lucascolley :)

@nstarman nstarman deleted the code/has_namespace branch June 9, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants